Deinitialize the pin definition #10940

mjm2017 · 2019-07-01T19:19:57Z

Without freeing the DAC, the pin will continue to be configured as DAC. Which make it impossible to change it from AnalogOut to anything else.

Description

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

Release Notes

Without freeing the DAC, the pin will continue to be configured as DAC. Which make it impossible to change it from Analogout to anything else. Check this code that allow you to change the AnalogOut to DigitalOut <<code>> #include "mbed.h" class myAnalog : public AnalogOut{ public: myAnalog(PinName myname); ~myAnalog(); PinName _myname; }; myAnalog::myAnalog(PinName myname):AnalogOut(myname), _myname(myname){ ; } myAnalog::~myAnalog(){ analogout_free(&this->_dac); } myAnalog *x=0; DigitalOut *xx=0; void do_something_analog() { x=new myAnalog(PA_4); double g=0.0; while(g<0.5){ x->write(g); g=g+0.1; wait_ms(50); } if (x!=0){ delete x; x=0; } } void do_something_digital() { xx=new DigitalOut(PA_4); for(int i=0;i<10;i++){ *xx = 1; wait_ms(50); *xx = 0; wait_ms(50); } if (xx!=0){ delete xx; xx=0; } } int main() { printf("\nAnalog loop example\n"); while(1) { do_something_digital(); do_something_analog(); } } <</code>>

ciarmcom · 2019-07-01T21:00:32Z

@mjm2017, thank you for your changes.
@ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

0xc0170 · 2019-07-02T11:54:55Z

drivers/AnalogOut.h

    {
-        // Do nothing
+        // deinitialize the pin configuration totally. This should allow chaining the definition of the pin
+          analogout_free(&this->_dac);


please fix the code style - see travis astyle failure

Sorry, But this is my first pull request. Please feel free to change my patch to whatever works for you by that I learn also to not make that mistake again.
thanks.

I tested this with Nucleo-L476RG.
The example sent with the patch .. I re-write it again:
#include "mbed.h"

class myAnalog : public AnalogOut{
public:

myAnalog(PinName myname); ~myAnalog();

PinName _myname;
};

myAnalog::myAnalog(PinName myname):AnalogOut(myname),
_myname(myname){
;

}
myAnalog::~myAnalog(){
analogout_free(&this->_dac);
}

myAnalog *x=0;
DigitalOut *xx=0;
void do_something_analog() {
x=new myAnalog(PA_4);
double g=0.0;
while(g<0.5){
x->write(g);
g=g+0.1;
wait_ms(50);
}
if (x!=0){
delete x;
x=0;
}
}

void do_something_digital() {
xx=new DigitalOut(PA_4);
for(int i=0;i<10;i++){
*xx = 1;
wait_ms(50);
*xx = 0;
wait_ms(50);
}
if (xx!=0){
delete xx;
xx=0;
}
}

int main()
{
printf("\nAnalog loop example\n");
while(1) {
do_something_digital();
do_something_analog();

}
}

No worries, worth reading: https://os.mbed.com/docs/mbed-os/v5.13/contributing/style.html

Can you please align the comment and the code line 143? Otherwise looks good

Dear Martin,
I cannot find this note as you mentioned (" Note: This is not currently used in the mbed-drivers")
Please write down for me the link to the file.
Notice that My change is only for NUCLEO-L476RG

0xc0170 · 2019-07-02T12:01:20Z

This should be fixed in few other drivers as well. There are implications - free is either not implemented or if it is, we do not know what can happen once it is invoked in drivers. This change should have tests with it. How did you test it (what target do you use) ?

I would get this to the next minor release.

Remove also the comment: * Note: This is not currently used in the mbed-drivers note from analogout_api.c

Without freeing the DAC, the pin will continue to be configured as DAC. Which make it impossible to change it from AnalogOut to anything else.

Initialization for different use should take care of it in this use case (if you do AnalogIn on the pin, it should be reconfigured). Still though free should be invoked.

0xc0170 · 2019-08-09T13:14:38Z

Remove also the comment: * Note: This is not currently used in the mbed-drivers note from analogout_api.c

Still to do

Travis fails , please review astyle job

mjm2017

Comments fixed

0xc0170 · 2019-08-23T14:17:45Z

astyle job still fails, please review.

@ARMmbed/mbed-os-hal Please review

fkjagodzinski

As @0xc0170 already asked -- please update the HAL header:

mbed-os/hal/analogout_api.h

Line 51 in 7e179f5

* Note: This is not currently used in the mbed-drivers

fkjagodzinski · 2019-08-27T10:15:26Z

drivers/AnalogOut.h

+        /** Deinitialize the pin configuration totally. 
+         *\ This should allow changing the definition of the pin
+         */
+          analogout_free(&this->_dac);


Suggested change

analogout_free(&this->_dac);

analogout_free(&_dac);

Private members are already marked with a _ prefix; no need to use this pointer here.

0xc0170 · 2019-09-18T12:27:55Z

@fkjagodzinski Please review

0xc0170 · 2019-09-20T07:18:45Z

CI restarted

mbed-ci · 2019-09-20T11:01:59Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

ciarmcom requested review from a team July 1, 2019 21:00

ciarmcom added the needs: review label Jul 1, 2019

bulislaw approved these changes Jul 2, 2019

View reviewed changes

0xc0170 requested changes Jul 2, 2019

View reviewed changes

0xc0170 added needs: work and removed needs: review labels Jul 2, 2019

Update AnalogOut.h

c6fbcb4

0xc0170 requested a review from a team August 9, 2019 13:13

Update AnalogOut.h

656f4b8

mjm2017 commented Aug 12, 2019

View reviewed changes

fkjagodzinski suggested changes Aug 27, 2019

View reviewed changes

Remove Note as requested

358df27

mjm2017 requested review from 0xc0170 and removed request for a team September 16, 2019 19:53

Mariwan added 2 commits September 16, 2019 21:57

remove extra space

47c3083

Update AnalogOut.h

5b951d4

0xc0170 requested a review from fkjagodzinski September 18, 2019 12:15

0xc0170 approved these changes Sep 18, 2019

View reviewed changes

0xc0170 added needs: review release-version: 5.15.0-rc1 and removed needs: work labels Sep 18, 2019

0xc0170 added needs: CI and removed needs: review labels Sep 19, 2019

0xc0170 added ready for merge and removed needs: CI labels Sep 20, 2019

0xc0170 merged commit 99c19b5 into ARMmbed:master Sep 20, 2019

0xc0170 removed the ready for merge label Sep 20, 2019

adbridge added release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 and removed release-version: 5.15.0-rc1 labels Oct 3, 2019

0xc0170 added release-version: 5.15.0-rc1 and removed release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 labels Nov 19, 2019

Deinitialize the pin definition #10940

Deinitialize the pin definition #10940

Uh oh!

Conversation

mjm2017 commented Jul 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Reviewers

Release Notes

Uh oh!

ciarmcom commented Jul 1, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Jul 2, 2019

Uh oh!

0xc0170 commented Aug 9, 2019

Uh oh!

mjm2017 left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Aug 23, 2019

Uh oh!

fkjagodzinski left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Sep 18, 2019

Uh oh!

0xc0170 commented Sep 20, 2019

Uh oh!

mbed-ci commented Sep 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mjm2017 commented Jul 1, 2019 •

edited

Loading